Fix EC JWKS authentication support by wheresNasha · Pull Request #6106 · opensearch-project/security

wheresNasha · 2026-04-22T17:49:04Z

Description

This PR fixes EC-based JWKS authentication support as part of issue #6045.

Category : Bug fix
Why these changes are required?
EC-based JWT verification using JWKS was not handled correctly, leading to authentication failures for EC-signed tokens.
What is the old behavior before changes and new behavior after changes?
Old behavior:
EC-signed JWTs using JWKS could fail validation due to missing or incorrect key handling.
New behavior:
EC JWKS keys are now properly parsed and used for JWT verification, enabling successful authentication for EC-signed tokens.

Issues Resolved

Is this a backport? If so, please add backport PR # and/or commits #, and remove backport-failed label from the original PR.

Do these changes introduce new permission(s) to be displayed in the static dropdown on the front-end? If so, please open a draft PR in the security dashboards plugin and link the draft PR here

Testing

Validated locally using EC-signed JWTs with JWKS
Tested token verification flow end-to-end
Confirmed backward compatibility with existing RSA-based flows

Check List

New functionality includes testing
New functionality has been documented
New Roles/Permissions have a corresponding security dashboards plugin PR
API changes companion pull request created
Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

codecov · 2026-04-27T17:09:45Z

Codecov Report

❌ Patch coverage is 60.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.81%. Comparing base (b8bd644) to head (4143730).
⚠️ Report is 14 commits behind head on main.

Files with missing lines	Patch %	Lines
.../security/auth/http/jwt/keybyoidc/JwtVerifier.java	60.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6106      +/-   ##
==========================================
+ Coverage   74.78%   74.81%   +0.03%     
==========================================
  Files         447      447              
  Lines       28428    28478      +50     
  Branches     4318     4332      +14     
==========================================
+ Hits        21260    21307      +47     
- Misses       5172     5177       +5     
+ Partials     1996     1994       -2

Files with missing lines	Coverage Δ
.../security/auth/http/jwt/keybyoidc/JwtVerifier.java	`75.40% <60.00%> (-1.79%)`	⬇️

... and 26 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

cwperks · 2026-05-01T18:15:44Z

@wheresNasha Can you please fix the Code Hygiene errors? Run ./gradlew spotlessApply and commit the changes.

cwperks · 2026-05-01T18:16:45Z

-        if (key.getClass() == OctetSequenceKey.class) {
+        if (key instanceof OctetSequenceKey) {
            result = new DefaultJWSVerifierFactory().createJWSVerifier(jwt.getHeader(), key.toOctetSequenceKey().toSecretKey());
+        } else if (key instanceof RSAKey) {


Can we write tests to hit all of these branches?

I have already addded test coverage for the missing EC (ECKey) branch here.

I also looked into adding OCT (OctetSequenceKey) coverage, but the current JWKS authentication flow does not appear to successfully authenticate symmetric OCT keys in the existing test infrastructure (extractCredentials() returns null), so that test is currently failing.

RSA coverage is already present, so this PR focuses on covering the missing EC path.

cwperks · 2026-05-01T18:18:04Z

+                    .createJWSVerifier(jwt.getHeader(), key.toECKey().toECPublicKey());
        } else {
-            result = new DefaultJWSVerifierFactory().createJWSVerifier(jwt.getHeader(), key.toRSAKey().toRSAPublicKey());
+            throw new IllegalArgumentException("Unsupported JWK key type: " + key.getClass());


Not sure if it makes sense but should we keep the fallback logic in place?

Any idea what the entire universe of key types can be?

I leaned toward explicit key type handling here since failing closed felt safer than broadly falling back and potentially masking unsupported JWK types.
From what I understand, Nimbus JWK currently supports symmetric (oct), RSA, EC, and additional types like OKP.

If preserving backward compatibility with prior RSA fallback makes more sense here, I’m happy to adjust it.

Signed-off-by: Sakshi Nasha <sakshinasha11@gmail.com>

github-actions · 2026-05-02T05:59:44Z

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ Recommended focus areas for review Missing Key Type The new `else` branch throws `IllegalArgumentException` for unsupported JWK key types. This exception is not caught in the calling code and may propagate as an unhandled runtime exception rather than a proper `BadCredentialsException`, potentially leaking internal details to callers. } else { throw new IllegalArgumentException("Unsupported JWK key type: " + key.getClass()); } Hardcoded Private Key The EC private key `EC_1_D` is hardcoded as a static string. While this is test code, ensure these values are not accidentally reused in any non-test context and that the key material is clearly documented as test-only. static final String EC_1_D = "lkTOA6MmmMDUg44V0coRAHbB5Zw-0748N8l8EOK8-5A"; Limited Test Coverage The new EC test only validates a happy path (valid EC-signed JWT). There are no tests for invalid EC signatures, unsupported EC curves, or missing EC key scenarios, which would provide stronger confidence in the fix. @Test public void testJwksAuthenticationWithEC() throws Exception { MockJwksServer mockJwksServer = new MockJwksServer(TestJwk.Jwks.ALL); try { Settings settings = Settings.builder().put("jwks_uri", mockJwksServer.getJwksUri()).build(); HTTPJwtKeyByJWKSAuthenticator jwtAuth = new HTTPJwtKeyByJWKSAuthenticator(settings, null); AuthCredentials creds = jwtAuth.extractCredentials( new FakeRestRequest(ImmutableMap.of("Authorization", TestJwts.MC_COY_SIGNED_EC_1), new HashMap<>()).asSecurityRequest(), null ); Assert.assertNotNull(creds); assertThat(creds.getUsername(), is(TestJwts.MCCOY_SUBJECT)); } finally { mockJwksServer.close(); } }

github-actions · 2026-05-02T06:00:00Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Wrap unsupported key type in declared exception The `IllegalArgumentException` thrown for unsupported key types is not declared in the method signature and may propagate unexpectedly to callers. Since the method already throws `BadCredentialsException` and `JOSEException`, the unsupported key type should be wrapped in one of those to maintain consistent error handling. src/main/java/org/opensearch/security/auth/http/jwt/keybyoidc/JwtVerifier.java [113-115] } else { - throw new IllegalArgumentException("Unsupported JWK key type: " + key.getClass()); + throw new BadCredentialsException("Unsupported JWK key type: " + key.getClass()); } Suggestion importance[1-10]: 6 __ Why: The `IllegalArgumentException` is unchecked and not declared in the method signature, while `BadCredentialsException` is already declared and would provide more consistent error handling for callers. This is a valid improvement for API consistency.	Low
General	Add attribute count assertion in EC test The EC authentication test only checks the username but does not verify the number of attributes like the RSA/OCT test does. This may miss regressions in claims extraction for EC-signed tokens. Consider adding an assertion on `creds.getAttributes().size()` to ensure claims are properly parsed. src/test/java/org/opensearch/security/auth/http/jwt/keybyoidc/HTTPJwtKeyByJWKSAuthenticatorTest.java [70-71] Assert.assertNotNull(creds); assertThat(creds.getUsername(), is(TestJwts.MCCOY_SUBJECT)); +assertThat(creds.getAttributes().size(), is(4)); Suggestion importance[1-10]: 4 __ Why: Adding an assertion on `creds.getAttributes().size()` would improve test coverage for EC-signed tokens, but this is a minor test completeness improvement that doesn't affect production code correctness.	Low

wheresNasha · 2026-05-02T06:06:22Z

@cwperks Thanks for the helpful guidance!
I’ve run ./gradlew spotlessApply to fix the formatting issues and have addressed the review comments as well.

Please let me know your thoughts, and if everything looks good from your side, I can rebase the branch.
I’d also appreciate it if CI could be triggered
Thanks!

wheresNasha requested review from DarshitChanpura, RyanL1997, cwperks, derek-ho, finnegancarroll, nibix, reta, shikharj05 and willyborankin as code owners April 22, 2026 17:49

cwperks reviewed May 1, 2026

View reviewed changes

Fix EC JWKS authentication support

4143730

Signed-off-by: Sakshi Nasha <sakshinasha11@gmail.com>

wheresNasha force-pushed the jwt-ec-jwks-fix branch from cbae25a to 4143730 Compare May 2, 2026 05:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix EC JWKS authentication support#6106

Fix EC JWKS authentication support#6106
wheresNasha wants to merge 1 commit intoopensearch-project:mainfrom
wheresNasha:jwt-ec-jwks-fix

wheresNasha commented Apr 22, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Apr 27, 2026 •

edited

Loading

Uh oh!

cwperks commented May 1, 2026

Uh oh!

cwperks May 1, 2026

Uh oh!

wheresNasha May 2, 2026 •

edited

Loading

Uh oh!

cwperks May 1, 2026

Uh oh!

wheresNasha May 2, 2026

Uh oh!

github-actions Bot commented May 2, 2026

Uh oh!

github-actions Bot commented May 2, 2026

Uh oh!

wheresNasha commented May 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

wheresNasha commented Apr 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Issues Resolved

Testing

Check List

Uh oh!

codecov Bot commented Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

cwperks commented May 1, 2026

Uh oh!

cwperks May 1, 2026

Choose a reason for hiding this comment

Uh oh!

wheresNasha May 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cwperks May 1, 2026

Choose a reason for hiding this comment

Uh oh!

wheresNasha May 2, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented May 2, 2026

PR Reviewer Guide 🔍

Uh oh!

github-actions Bot commented May 2, 2026

PR Code Suggestions ✨

Uh oh!

wheresNasha commented May 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

wheresNasha commented Apr 22, 2026 •

edited

Loading

codecov Bot commented Apr 27, 2026 •

edited

Loading

wheresNasha May 2, 2026 •

edited

Loading